-
Notifications
You must be signed in to change notification settings - Fork 407
feat: identities getting started #2103
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
TODO:
|
Feedback from @piotrmsc
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Structurally it's going in the right direction, but I think we need a bit more work on the content side!
Following again blindly "step by step" : When I try logout I am getting 404 after logout call as well, in ory tunnel I see calls and redirects but in the end I end up with 404 from CF blocking rule, do I need to configure something extra? |
docs/identities/get-started/_common/code-examples/js/session/check-session.js
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Huge PR!
I think we are very close to getting this one perfect and will be a great basis for getting started!
I did not test all the code examples but the ones I tested were straightforward enough - maybe you can create an issue to add automated tests for these examples later on? I think long-term would be good avoid drift.
I also noticed you added some neat new(?) components, love them!
Please track adding a "custom components" section to the README in a followup issue - so others know how to use these components? It is mostly clear from this example, but it could save us some time in the future - you don't need to do it in this PR though.
All in all I had mostly some minor style issues, nothing major. Let me know where you had followup questions, once we have this resolved let's
I agree with the idea of tests,however, to have them running and testing the code examples they would need to be code-complete and currently are not (see my previous review comments on them). Which for me would make sense to have getting-started code examples as code complete with option to collapse lines of code that are not ory-specific. @christiannwamba what's your take on it? |
Co-authored-by: Vincent <[email protected]>
Co-authored-by: Vincent <[email protected]>
Co-authored-by: Vincent <[email protected]>
Co-authored-by: Vincent <[email protected]>
3906aef
to
e9d1b70
Compare
Related Issue or Design Document
Checklist
If this pull request addresses a security vulnerability,
I confirm that I got approval (please contact [email protected]) from the maintainers to push the changes.
Further comments